feat: add workspace/didChangeWatchedFiles support#1408
feat: add workspace/didChangeWatchedFiles support#1408sebthom merged 1 commit intoeclipse-lsp4e:mainfrom
workspace/didChangeWatchedFiles support#1408Conversation
|
@angelozerr can you please check if this is what you need? You can download an update site with this PR from https://github.com/eclipse-lsp4e/lsp4e/actions/runs/19482961536/artifacts/4607414883 |
|
@sebthom I would like to share my experience with this feature that I have implemented in LSP4IJ. I suggest really that you support glob pattern to avoid having performance problem with language servers which receive some non expected files. Supporting glob pattern is not trivial. I think you could use a lot of code I have written for IJ in https://github.com/redhat-developer/lsp4ij/tree/main/src/main/java/com/redhat/devtools/lsp4ij/features/files You can find tests at https://github.com/redhat-developer/lsp4ij/tree/main/src/test/java/com/redhat/devtools/lsp4ij/features/files (which are the same than vscode) The main idea is to work with https://github.com/redhat-developer/lsp4ij/blob/main/src/main/java/com/redhat/devtools/lsp4ij/features/files/watcher/FileSystemWatcherManager.java See registration of watcher at https://github.com/redhat-developer/lsp4ij/blob/96db3b209cc09393af638bc1ff5b1ec92a2adb6c/src/main/java/com/redhat/devtools/lsp4ij/LanguageServerWrapper.java#L949 |
|
@sebthom I am sorry https://github.com/jbosstools/jbosstools-quarkus has been give up, I will not have time to test your PR |
|
@angelozerr I am trying to integrate the files from lsp4ji as you suggested. Can we normalize the license header to the style that is used in all files of LSP4E? LSP4E license header style: /*******************************************************************************
* Copyright (c) <YEAR> Red Hat Inc. and others.
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* <AUTHOR> (Red Hat Inc.) - initial implementation
*******************************************************************************/The LSP4IJ project seems to use slightly different license headers in different files: /*******************************************************************************
* Copyright (c) 2024 Red Hat, Inc.
* Distributed under license by Red Hat, Inc. All rights reserved.
* This program is made available under the terms of the
* Eclipse Public License v2.0 which accompanies this distribution,
* and is available at https://www.eclipse.org/legal/epl-v20.html
*
* Contributors:
* Red Hat, Inc. - initial API and implementation
*******************************************************************************/
class FileSystemWatcherManager/*******************************************************************************
* Copyright (c) 2019 Red Hat Inc. and others.
* All rights reserved. This program and the accompanying materials
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-v20.html
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Red Hat Inc. - initial API and implementation
*******************************************************************************/
class PathPatternMatcherI would like to avoid having a zoo of unicorn copyright statements in LSP4E. @mickaelistria maybe you can also say something about this? |
I am supper happy that you like the idea to use my code, That's great!
It is my code, so for me I have no problem that you do any change. If license allows that, feel free to change anything.
|
|
@angelozerr nice! What about the year 2019 in PathPatternMatcher. Was this a copy and paste error, or is this really an old file from six years ago? |
|
No it is an old file that I have written for lemminx if I remember. |
|
@sebthom an another point which is very important is that your resource listener can be called a lot of time for instance if you get a lot of files from git. Perhaps it is not the case with eclipse but with IJ it was the case. But keep in mind that your listener can be called a lot of time with a lot of files. In this case the glob pattern support that I have written which use Java IO glob can be slow. It is one reason why it is important to do thoses matches in a thread |
|
Thanks for the insight! |
sebthom
changed the title
workspace/didChangeWatchedFiles support
| * Resource listener that translates Eclipse resource change events into LSP | ||
| * file watch events and dispatches them if the language server is still active | ||
| */ | ||
| private final class WatchedFilesListener implements IResourceChangeListener { |
There was a problem hiding this comment.
move to its own class file in the internal package?
There was a problem hiding this comment.
I don't think this would be very useful. The class is very specialist, not reusable elsewhere and relies on multiple internal fields of LanguageServerWrapper (dispatcher, context, fileSystemWatcherManager).
There was a problem hiding this comment.
I think it can be useful since LanguageServerWrapper is a big file. You could create WatchedFilesListener without modifier in the same package than LanguageServerWrapper
There was a problem hiding this comment.
For that LanguageServerWrapper is in the wrong package. I would not want to pollute the root package with a "random" WatchedFilesListener class file that is an internal listener implementation (like LSFileBufferListener or WorkspaceFolderListener) of LanguageServerWrapper. If LoC of LanguageServerWrapper is a concern we should better think about moving all the generic utility methods like getRootCauseMessage/getThrowableMessage/disconnectTextFileBuffer/supportsWorkspaceFolders etc. out of LanguageServerWrapper but I would do this in a dedicated PR focused on refactoring/cleanup of the class.
3 participants
Fixes #547